Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: update UnixFS/MFS with auto sharding #8114

Closed
wants to merge 18 commits into from

Conversation

schomatis
Copy link
Contributor

@schomatis schomatis commented May 7, 2021

TODO:

Closes #8106.
Closes #8148.

@schomatis schomatis requested a review from aschmahmann May 7, 2021 20:11
@schomatis schomatis self-assigned this May 7, 2021
@schomatis schomatis marked this pull request as draft May 7, 2021 23:00
@schomatis schomatis removed their assignment May 7, 2021
@schomatis schomatis self-assigned this May 18, 2021
@schomatis schomatis force-pushed the feat/deps/update-unixfs-mfs-auto-sharding branch from 01639e4 to 811ea25 Compare May 19, 2021 14:48
achingbrain added a commit to ipfs/js-ipfs-unixfs that referenced this pull request Sep 12, 2021
js counterpart to ipfs/kubo#8114

Changes the `shardSplitThreshold` parameter to mean the size of the
final DAGNode (including link names, sizes, etc) instead of the
number of entries in the directory.

Fixes: #149

BREAKING CHANGE: `shardSplitThreshold` now refers to node size, not number of entries
Comment on lines 318 to +321
// TEMP: setting global sharding switch here
uio.UseHAMTSharding = cfg.Experimental.ShardingEnabled
uio.HAMTShardingSize = int(256 * units.KiB)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like this should live in go-unixfs since we'd like people to be using the automatic sharding unless they opt not to

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As it stands they will be using automatic sharding by default for go-ipfs, not for go-unixfs as a standalone library (independent of BitSwap, which is the one who introduce this restriction in the first place).

Do we also want this to be a default for go-unixfs as a library? (In that case we would set the HAMTShardingSize default value to 256KiB in unixfs and not here.)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we also want this to be a default for go-unixfs as a library? (In that case we would set the HAMTShardingSize default value to 256KiB in unixfs and not here.)

Yes, I think we should just put this in go-unixfs. If at some point that library starts taking sharding sizes in a non-global fashion we can deal with it there. The point of defaults is to give people something sane to start with if they're not deeply "in the know" so we might as well give it to go-unixfs consumers as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moving to UnixFS.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added in ipfs/go-unixfs@83ad983.

Will remove this after we update UnixFS/MFS dependencies with latest fixes.

@aschmahmann aschmahmann force-pushed the feat/deps/update-unixfs-mfs-auto-sharding branch from 5983810 to a369655 Compare October 14, 2021 00:02
@schomatis
Copy link
Contributor Author

Sharness are passing which is encouraging. Rebasing on master.

@aschmahmann
Copy link
Contributor

Closed by #8547

@schomatis schomatis deleted the feat/deps/update-unixfs-mfs-auto-sharding branch April 27, 2022 14:52
achingbrain added a commit to ipfs/js-ipfs-unixfs that referenced this pull request Feb 9, 2023
js counterpart to ipfs/kubo#8114

Changes the `shardSplitThreshold` parameter to mean the size of the final DAGNode (including link names, sizes, etc) instead of the number of entries in the directory.

Fixes: #149

BREAKING CHANGE: the `shardSplitThreshold` option has changed to `shardSplitThresholdBytes` and reflects a DAGNode size where sharding might kick in
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Tracking issue for UnixFS automatic unsharding Tracking issue for UnixFS automatic sharding
2 participants